Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ADD] #57 allow higher value overtime for holidays #81

Merged
merged 4 commits into from
Jan 22, 2024

Conversation

hbrunn
Copy link
Contributor

@hbrunn hbrunn commented Dec 11, 2023

No description provided.

@albig
Copy link
Member

albig commented Dec 14, 2023

Vielen Dank. Es funktioniert schon was.

Folgende Anmerkungen habe ich nach einem ersten Test:

  1. aktuell funktioniert es "nur" bei Feiertagen, richtig?
  2. Den initialen Faktor "0" beim Mitarbeiter find ich ok (nach etwas Nachdenken). Wir müssen darüber ja auch feststellen, ob der globale Wert überschrieben werden soll. "Leer" wäre intuitiver. Aber das geht bei dem Feld-Typ sicher nicht.
  3. In der Overtime-Tabelle sieht man im Backend keinen Unterschied, keinen Tag zum Extra/Adjustment-Eintrag
  4. Ändert man einen Anwesenheits-Eintrag, wird ein weiterer Overtime-Eintrag angelegt - auch wenn da schon einer vorhanden war.
  5. Es fehlt noch die Möglichkeit, dass man am Anwesenheits-Eintrag die Höherwertung aktivieren kann.

@hbrunn
Copy link
Contributor Author

hbrunn commented Dec 18, 2023

  1. es sollte eigentlich auch das Wochenende gehen. Wie simulierst Du das?
  2. ja, null-Werte in der Datenbank werden bei floats immer 0 - fur bessere UX erst eine checkbox die das Feld sichtbar macht, wenn aus gilt der Company-Faktor?
  3. das verstehe ich nicht
  4. ja, habe ich auf dem Schirm
  5. also die manuellen Anwesenheiten? Setzen wir da einfach eine Checkbox in die Liste, selbe Regeln wie beim Wizard?

@albig
Copy link
Member

albig commented Dec 18, 2023

  1. Bisschen rätselhaft. Beim ersten Versuch hatte er für den Sonntag (z.B. 17.12.2023) keinen zusätzlicher Eintrag in hr_attendance_overtime angelegt. An einem fiktiven Feiertag (z.B. 18.12.2023) aber schon. Jetzt legt er für jeden Tag die Zusatzstunden an. Bzw. zieht sie auch doppelt ab. Inhaltlich macht das grad keinen Sinn :-(
    2023-12-18_10-42

  2. Welche Zeilen durch Höherwertung entstanden sind und welche "normale" Überstunden sind, ist nicht ersichtlich. Hier wäre ein Tag o.ä. wünschenswert, wie es z.B. hr_attendanc_overtime_manual macht.

  3. Das führt aktuell zu der Verwirrung in 1., nehm ich an.

  4. Ja, ich denke in der Liste der Anwesenheiten wäre eine Checkbox rightig. Wenn die aktiviert wird, wird der hinterlegte "Holiday Overtime Faktor" angewendet.

  5. Aktuell kann ich keine "Attendances" löschen, obwohl beim Mitarbeiter der "Holiday Overtime Faktor" gesetzt ist (egal ob 0,00 oder 1,00).

Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/odoo/http.py", line 654, in _handle_exception
    return super(JsonRequest, self)._handle_exception(exception)
  File "/usr/lib/python3/dist-packages/odoo/http.py", line 301, in _handle_exception
    raise exception.with_traceback(None) from new_cause
ValueError: Invalid field 'holiday_overtime_factor' on model 'hr.employee.public'

@hbrunn hbrunn force-pushed the 15.0-57-holiday_overtime_factor branch from efc400a to d7944b1 Compare December 20, 2023 12:37
@hbrunn
Copy link
Contributor Author

hbrunn commented Dec 20, 2023

@albig jetzt bin ich recht zufrieden, weitere Fragen:

  1. Wie gehen wir um mit zwei Einträgen am selben Tag, einem mit Faktor eingeschaltet und einem ohne? Problem ist dass Upstream keinen Unterschied macht von welchem Attendance-Eintrag eine Overtime herkommt, und ich auf diesen Mechanismus aufsetze. Momentan habe ich es so implementiert, wenn eine der Anwesenheiten das Flag gesetzt hat, gilt das für alle Anwesenheiten des Tages. Akzeptabel? Hoffentlich, weil das auseinander zu ziehen konzeptuelle Umbauten erfordert.
  2. Es scheint mir noch sinnvoll dass die Flag-Spalte nur zu sehen ist wenn Company oder Employee einen Faktor gesetzt haben? Ich habe das Flag bewusst 'dumm' gelassen (keine Berechnung auf Zeilenniveau ob das eine Anwesenheit ist bei der das überhaupt in Frage kommt) um keine Performance-Probleme bei grossen Listen zu erzeugen. Vielleicht ist eine Fehlermeldung noch gut wenn das Flag an einem normalen Tag gesetzt wird? Stand jetzt ist wenn Du das setzt und es ist weder Feiertag noch Wochenende, passiert halt nichts. Schnell, aber ggf irreführend?

@hbrunn hbrunn changed the title [POC][ADD] #57 allow higher value overtime for holidays [ADD] #57 allow higher value overtime for holidays Dec 20, 2023
@hbrunn hbrunn force-pushed the 15.0-57-holiday_overtime_factor branch from d7944b1 to 97bff11 Compare December 20, 2023 16:18
@codecov-commenter
Copy link

codecov-commenter commented Dec 20, 2023

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (172b932) 79.73% compared to head (629cf13) 89.17%.

❗ Current head 629cf13 differs from pull request most recent head 6742132. Consider uploading reports for the commit 6742132 to get more accurate results

Files Patch % Lines
verdigado_attendance/models/hr_attendance.py 87.50% 2 Missing and 1 partial ⚠️
verdigado_attendance/models/res_users.py 83.33% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             15.0      #81      +/-   ##
==========================================
+ Coverage   79.73%   89.17%   +9.43%     
==========================================
  Files          16       13       -3     
  Lines         306      157     -149     
  Branches       17       23       +6     
==========================================
- Hits          244      140     -104     
+ Misses         61       16      -45     
  Partials        1        1              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@albig
Copy link
Member

albig commented Jan 15, 2024

Die Checkbox ist total prima.

2024-01-15_13-03

Wenn ich das richtig sehe, wird der Custom-Faktor aber immer noch verwendet, wenn man die Checkbox deaktiviert.

Doch, das geht. Sorry.

@albig
Copy link
Member

albig commented Jan 15, 2024

Kleinigkeit:

  • Employee: Custom Holiday Overtime Factor == 1.50
  • Company: Holiday Overtime Factor == 1.20

Es wird richtig gerechnet mit 1,50. Aber im Text steht der Company-Faktor:

2024-01-15_13-19

@hbrunn hbrunn force-pushed the 15.0-57-holiday_overtime_factor branch from 00b8f43 to 609a146 Compare January 15, 2024 15:33
@hbrunn
Copy link
Contributor Author

hbrunn commented Jan 15, 2024

was die Anzeige angeht: Hast Du zuerst Zeit geschrieben mit dem Company-Faktor, danach den mitarbeiterspezifischen Faktor aktiviert, und dann die Zeit noch einmal angepasst? In dem Fall schreibe ich das Label nicht nochmal neu, sondern passe nur die geschriebene Zeit an.

@albig
Copy link
Member

albig commented Jan 16, 2024

Hab den Höherwertungs-Hinweis im Wizard gefunden :-)

2024-01-16_10-44

Aber leider wird das Setting nicht übernommen:

2024-01-16_10-51

@albig
Copy link
Member

albig commented Jan 16, 2024

Das Löschen von Datensätzen aus hr_attendance funktioniert mit dem Code-Stand nicht mehr:

2024-01-16 10:37:42,985 1 INFO lv-nds odoo.models.unlink: User #2 deleted hr.attendance records with IDs: [137] 
2024-01-16 10:37:43,076 1 INFO lv-nds odoo.models.unlink: User #2 deleted hr.attendance.overtime records with IDs: [92] 
2024-01-16 10:37:43,107 1 WARNING lv-nds odoo.http: Datensatz existiert nicht oder kann nicht gefunden werden.
(Datensatz: hr.attendance(137,), Benutzer: 2) 

2024-01-16_11-39

2024-01-16_11-41
2024-01-16_11-40

Liegt das an meinem Setup?

@hbrunn
Copy link
Contributor Author

hbrunn commented Jan 22, 2024

danke, Loschen ist repariert, das Flag schaue ich mir jetzt an

@hbrunn
Copy link
Contributor Author

hbrunn commented Jan 22, 2024

danke, Loschen ist repariert, das Flag schaue ich mir jetzt an

das ist jetzt auch fertig

Copy link
Member

@albig albig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Die offenen Punkte sind behoben. Für mich sieht es vollständig aus und ich übernehme den PR.

@albig albig merged commit 9c2f8ee into verdigado:15.0 Jan 22, 2024
4 checks passed
@albig albig linked an issue Jan 22, 2024 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Höherwertung von Überstunden an Wochenenden und Feiertagen
3 participants